Update Costing Documentation #1716
Conversation
|
|
||
| def build_caoh2_cost_param_block(blk): | ||
| blk.cost = pyo.Param( | ||
| blk.CaOH2_cost = pyo.Param( |
There was a problem hiding this comment.
Curious what inspired these changes to make cost more specific
There was a problem hiding this comment.
Mainly just personal preference - I thought it was a bit too ambiguous. There also wasn't any consistency as to what cost meant. In some scenarios, it referred to unit model costs and in others it referred to chemical costs. Evidently, this name change has resulted in some test failures that I need to fix. Now, unit costs should be consistently called unit_cost and chemical costs called chemical_cost
There was a problem hiding this comment.
In reality, it probably wasn't too problematic since I believe it will usually be paired with the corresponding docstring
There was a problem hiding this comment.
@MarcusHolly I wonder if we should just use unit_cost across the board. For chemicals, $ / kg cost is technically a unit cost. Any cost per unit of quantity can be rendered a unit_cost. For pumps, unit_cost would be something like $/W, and so on.
There was a problem hiding this comment.
I agree with this - thanks for the suggestion
| m.fs.costing.reverse_osmosis.membrane_cost.fix(30) | ||
| m.fs.costing.reverse_osmosis.high_pressure_membrane_cost.fix(50) | ||
| m.fs.costing.high_pressure_pump.cost.fix(53 / 1e5 * 3600) | ||
| m.fs.costing.high_pressure_pump.unit_cost.fix(53 / 1e5 * 3600) |
There was a problem hiding this comment.
@adam-a-a The best argument for updating the cost names is to make code more intuitive when flowsheets are updating the values of these variables.
There was a problem hiding this comment.
I propose unit_cost across the board so that is clear that the cost is in $ per some quantity, as opposed to some absolute cost.
|
|
||
| "Membrane replacement factor", ":math:`f_{replace}`", "``factor_membrane_replacement``", "0.2", ":math:`\text{yr}^{-1}`" | ||
| "Membrane cost", ":math:`C_{mem}`", "``membrane_unit_cost``", "56", ":math:`\text{USD}_{2018}\text{/m}^2`" | ||
| "Membrane cost", ":math:`C_{mem}`", "``membrane_cost``", "56", ":math:`\text{USD}_{2018}\text{/m}^2`" |
There was a problem hiding this comment.
Should instances of membrane_cost also be replaced with unit_cost?
| "**CaOH2 mixer** (cost method = `cost_caoh2_mixer`)" | ||
| "Mixer unit cost", ":math:`C_{mix, CaOH2}`", "``unit_cost``", "873.911", ":math:`\text{USD}_{2018}\text{/kg/day}`" | ||
| "CaOH2 cost", ":math:`C_{CaOH2}`", "``CaOH2_cost``", "0.12", ":math:`\text{USD}_{2018}\text{/kg}`" |
There was a problem hiding this comment.
How should we handle this? Call the unit model cost and the chemical cost unit_COST ?
There was a problem hiding this comment.
Ugh-- another alternative can be unit_cost_CaOH2 or basically unit_cost appended or prepended with the specific item name (e.g., unit_cost_pump, unit_cost_antiscalant).
We should also probably check IDAES costing and try to be as consistent as we can across projects. For example, if IDAES already uses some convention for these sorts of costs (and the naming isn't too cryptic), we can use that convention.
There was a problem hiding this comment.
From a quick skim, I don't think there's an IDAES convention for these sorts of costs.
3 participants
Summary/Motivation:
The cost methods are missing from most of the documentation
Changes proposed in this PR:
Legal Acknowledgement
By contributing to this software project, I agree to the following terms and conditions for my contribution: